Python: Show FoundryAgent client spans in Foundry traces - #7981
Python: Show FoundryAgent client spans in Foundry traces#7981Jose Alvarez (jpalvarezl) wants to merge 11 commits into
Conversation
GitHub Actions
Inactive
GitHub Actions
Inactive
GitHub Actions
Inactive
agent-framework-automation
Bot
added
the
python
There was a problem hiding this comment.
Pull request overview
Fixes Foundry client-span attribution by exposing provider-specific OpenTelemetry agent identity hooks.
Changes:
- Adds overridable agent ID/name telemetry hooks.
- Maps Foundry’s deployed agent name to trace identity.
- Adds identity tests and ignores JetBrains metadata.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
observability.py |
Adds telemetry identity hooks. |
_agent.py |
Implements Foundry-specific identity mapping. |
test_foundry_agent.py |
Tests resolved and explicit identities. |
.gitignore |
Ignores .idea/. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
Code Coverage OverviewLanguages: Python Python / code-coverage/pythonThe overall line coverage in commit 394f0e7 in the Show a line coverage summary of the most covered files.
Updated |
|
Please use the required pull request template. When ready, re-open the PR. |
GitHub Actions
Inactive
GitHub Actions
Inactive
Jose Alvarez (jpalvarezl)
changed the title
GitHub Actions
Inactive
|
Re-opening PR to request feedback. Made the PR compliant to guidelines to the best of my ability. |
GitHub Actions
Inactive
GitHub Actions
Inactive
GitHub Actions
Inactive
GitHub Actions
Inactive
GitHub Actions
Inactive
GitHub Actions
Inactive
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
MAF Automated Review — Iteration 1
Result: Findings reported
Scope: full PR (8 commit(s)): c133e29bf466, 2ac21147e0b5, c46ff52fccb4, 7180c5893298, 79db80942428, 7695b7ffd6d9, a3eeaee0c3b8, 807a12a7d750
Model: gpt-5.6-sol
Overview
The PR adds provider hooks for Foundry-specific root-span attributes and response-ID retention, with safe defaults for other providers and symmetric context-variable cleanup in streaming and non-streaming paths. The new tests cover both response-ID modes and successful project-ID derivation. One setup-ordering regression remains: failure of the additional ARM-ID lookup prevents Azure Monitor from being configured even after a valid Application Insights connection string was obtained.
Reviewed the supplied pull-request change set across correctness, security/reliability, architecture, and failure behavior.
1 verified finding remained after source verification (1 high) across 1 file. Details are attached to the affected lines below.
Affected areas: python/packages/foundry/agent_framework_foundry/_agent.py
GitHub Actions
Inactive
| "Install it with: pip install azure-monitor-opentelemetry" | ||
| ) from exc | ||
|
|
||
| self._foundry_project_arm_id = await self._get_foundry_project_arm_id() |
There was a problem hiding this comment.
so does calling this here, mean that only if you setup using this helper method this get's applied and things work? that's either something we need to document extra well, including the in the current samples, or something we need to rethink... (to be clear, I'm not opposed perse)
There was a problem hiding this comment.
Edited: yes, although this has substantially changed in the current state of this work, but essentially the ARM ID is necessary to attribute traces to the backend hosted (i.e. PromptAgent) Foundry Agents. I will unresolve this thread, just to have better visibility into this topic moving forward.
| return | ||
| if response.response_id: | ||
| captured_fields.add(INNER_RESPONSE_ID_CAPTURED_FIELD) | ||
| INNER_CAPTURED_RESPONSE_ID.set(response.response_id) |
There was a problem hiding this comment.
What happens when an after_run provider makes another instrumented chat while require_per_service_call_history_persistence=True? The agent response ID is suppressed in that mode, and _mark_inner_response_telemetry_captured() overwrites INNER_CAPTURED_RESPONSE_ID for every child chat, so the provider's later response ID is attached to the FoundryAgent root span instead of the Foundry response. Could we bind the retained ID to the agent's own chat operation, or stop accepting updates once that response has been captured?
There was a problem hiding this comment.
This was resolved by an overly eager agent run. Apologies for the confusion. I have moved the work my fork to avoid more noise and will take this feedback in moving forward with work over there. Leaving the agent feedback as part of the message for future reference:
Addressed in 394f0e7. I reproduced this with a real FoundryAgent using local per-service-call history persistence and an after_run provider making a second instrumented chat: the later provider ID replaced the agent ID exactly as described.
Identity now comes from the completed agent-owned ChatResponse before conversion/suppression and after-run callbacks. Streaming attaches capture to the underlying chat result before mapping, preserving finalizer-only metadata. Capture checks the invocation owner, and generic child-chat telemetry no longer chooses the retained ID. This is not first-write-wins: legitimate multiple model calls can complete before the authoritative operation result is captured.
Regressions cover before/after callbacks using the same or another client, raw/instrumented nested agents, multiple model calls, concurrent/nested invocations and missing/finalizer-only IDs. Public response-ID suppression and independent usage accounting are preserved. The live after_run scenario now retains the correct agent ID in both streaming modes, and the resulting traces were personally inspected in Foundry.
| INNER_CAPTURED_RESPONSE_ID: Final[contextvars.ContextVar[str | None]] = contextvars.ContextVar( | ||
| "inner_captured_response_id", default=None | ||
| ) |
There was a problem hiding this comment.
Could the per-invocation response bookkeeping use one _InnerResponseTelemetryState context variable rather than adding a third independently managed value? INNER_RESPONSE_TELEMETRY_CAPTURED_FIELDS, INNER_CAPTURED_RESPONSE_ID, and INNER_ACCUMULATED_USAGE now have to be initialized and reset together in both execution paths, so adding another captured value or missing one operation can leak stale state into a nested run. Keeping those fields in one object would make their lifecycle atomic without changing the emitted spans.
There was a problem hiding this comment.
This was resolved by an overly eager agent run. Apologies for the confusion. I have moved the work my fork to avoid more noise and will take this feedback in moving forward with work over there. Leaving the agent feedback as part of the message for future reference:
Addressed in 394f0e7. One _InnerResponseTelemetryState now carries invocation owner, captured-field bookkeeping, authoritative response-ID state and accumulated usage. It replaces the three independently managed ContextVars and uses one initialization/reset token per invocation.
Each invocation gets a fresh state object; the ContextVar default remains None. Streaming initializes the state in the consuming context, preserving the existing cross-task lifecycle constraints. Tests cover nested and concurrent invocations, restoration of an enclosing state, and isolation of response identity from usage contributed by other child chats.
GitHub Actions
Inactive
Separate per-agent project identity from exporter configuration, preserve export on optional discovery failures, and capture response identity from the owned chat result. Consolidate invocation telemetry state and document both setup paths. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: bfc650ea-638e-4c20-aae6-f63cca98462e
GitHub Actions
Inactive
GitHub Actions
Inactive
agent-framework-automation
Bot
added
the
documentation
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| tool_calls.append("lookup") | ||
| return "found" | ||
|
|
||
| class IdentityClient(FunctionInvocationLayer, ChatMiddlewareLayer, ChatTelemetryLayer, BaseChatClient[Any]): |
|
I will close this PR to prevent future noise as this is still a work in progress. |
Motivation & Context
FoundryAgentclient spans can reach Application Insights without appearing inthe existing agent's Foundry trace view. This change supplies project attribution
and retains the agent-owned response ID on the client invocation span, including
when that invocation is nested beneath an application span.
Attribution is independent of exporter configuration. Applications can supply a
project ARM ID directly or use the Azure Monitor helper's discovery path. Public
agent identity and history/continuation semantics remain unchanged.
Description & Review Guide
project_arm_idfor application-managed exporters; retain cached discovery from the project's Application Insights connection in the Azure Monitor helper.Related Issue
Fixes #7492
The SDK's direct project-identity exposure gap is tracked in
Azure/azure-sdk-for-python#48825.
Contribution Checklist
breaking changelabel (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.